Skip to content

bazel: pass git to lint scripts in fix_lint#10813

Open
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:fix-lint-git-arg
Open

bazel: pass git to lint scripts in fix_lint#10813
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:fix-lint-git-arg

Conversation

@oharboe

@oharboe oharboe commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

bazelisk run //:fix_lint has been failing with bazel/tcl_lint_test.sh: line 10: $2: unbound variable: the lint scripts take the git binary as their second argument (their sh_test wrappers pass $(rootpath @git)), but the //:fix_lint driver invoked them with only the lint tool. Pass $(rootpath @git) through.

Verification

bazelisk run //:fix_lint completes with both lint passes running (verified on top of #10812, since the prebuilt ld.lld used by current master cannot start on an Ubuntu 26.04 host without libxml2.so.2 — the failure #10812 addresses).

Type of Change

  • Bug fix (build infrastructure)

  • I have run the relevant tests and they pass

  • My code follows the repository's formatting guidelines

  • I have signed my commits (DCO).

tcl_lint_test.sh and bzl_lint_test.sh take the git binary as their
second argument (as their sh_test wrappers pass it), but //:fix_lint
invoked them with only the lint tool, so 'bazelisk run //:fix_lint'
always failed with '$2: unbound variable'.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe oharboe requested a review from a team as a code owner July 4, 2026 13:42
@oharboe oharboe requested a review from precisionmoon July 4, 2026 13:42

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds @git as a dependency and passes it as a ninth positional parameter ($9) to the linting commands in bazel/fix_lint.sh. The reviewer suggests assigning positional parameters to descriptive named variables at the beginning of the script to improve readability and maintainability instead of using magic positional parameters like $9.

Comment thread bazel/fix_lint.sh Outdated
@oharboe

oharboe commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

@hzeller FYI

Comment thread bazel/fix_lint.sh Outdated
Comment thread bazel/fix_lint.sh Outdated
Review feedback: assign the positional parameters to named variables,
matching the style of the per-language lint scripts, and use the passed
git for the final status report.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@github-actions github-actions Bot added size/S and removed size/XS labels Jul 4, 2026
@oharboe oharboe requested a review from hzeller July 4, 2026 17:36

@hzeller hzeller left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants